Simplify parsing and printing attributes of function and arguments #683

mununki · 2022-10-15T10:21:26Z

Regarding issue rescript-lang/rescript#5735

This PR improves the parsing and printing attributes of functions and arguments so that it makes the implementation simple and clear for future development extensibility.

mununki · 2022-10-15T10:24:10Z

Parsing looks okay now, but the printing error is due to an infinite loop.

let f12 = @a (@b x) => 3
let f13 = @a @b (~x) => 3

  structure_item (tests/printer/expr/asyncAwait.res[97,2562+0]..[97,2562+24])
    Pstr_value Nonrec
    [
      <def>
        pattern (tests/printer/expr/asyncAwait.res[97,2562+4]..[97,2562+7])
          Ppat_var "f12" (tests/printer/expr/asyncAwait.res[97,2562+4]..[97,2562+7])
        expression (tests/printer/expr/asyncAwait.res[97,2562+13]..[97,2562+24])
          attribute "a" (tests/printer/expr/asyncAwait.res[97,2562+10]..[97,2562+12])
            []
          Pexp_fun
          Nolabel
          None
          pattern (tests/printer/expr/asyncAwait.res[97,2562+17]..[97,2562+18])
            attribute "b" (tests/printer/expr/asyncAwait.res[97,2562+14]..[97,2562+16])
              []
            Ppat_var "x" (tests/printer/expr/asyncAwait.res[97,2562+17]..[97,2562+18])
          expression (tests/printer/expr/asyncAwait.res[97,2562+23]..[97,2562+24])
            Pexp_constant PConst_int (3,None)
    ]
  structure_item (tests/printer/expr/asyncAwait.res[98,2587+0]..[98,2587+25])
    Pstr_value Nonrec
    [
      <def>
        pattern (tests/printer/expr/asyncAwait.res[98,2587+4]..[98,2587+7])
          Ppat_var "f13" (tests/printer/expr/asyncAwait.res[98,2587+4]..[98,2587+7])
        expression (tests/printer/expr/asyncAwait.res[98,2587+16]..[98,2587+25])
          attribute "a" (tests/printer/expr/asyncAwait.res[98,2587+10]..[98,2587+12])
            []
          attribute "b" (tests/printer/expr/asyncAwait.res[98,2587+13]..[98,2587+15])
            []
          Pexp_fun
          Labelled "x"
          None
          pattern (tests/printer/expr/asyncAwait.res[98,2587+17]..[98,2587+19])
            attribute "ns.namedArgLoc" (tests/printer/expr/asyncAwait.res[98,2587+18]..[98,2587+19])
              []
            Ppat_var "x" (tests/printer/expr/asyncAwait.res[98,2587+17]..[98,2587+19])
          expression (tests/printer/expr/asyncAwait.res[98,2587+24]..[98,2587+25])
            Pexp_constant PConst_int (3,None)
    ]

cristianoc · 2022-10-15T10:27:03Z

The body returned should be "3" in both cases.

cristianoc · 2022-10-15T10:27:58Z

Needs to always make progress, and put together a group of args.

mununki · 2022-10-15T10:28:37Z

expression (tests/printer/expr/asyncAwait.res[97,2562+23]..[97,2562+24])
Pexp_constant PConst_int (3,None)

          expression (tests/printer/expr/asyncAwait.res[97,2562+23]..[97,2562+24])
            Pexp_constant PConst_int (3,None)

I think this is the body returns "3"

mununki · 2022-10-15T16:50:15Z

tests/printer/expr/expected/asyncAwait.res.txt

+let f12 = @a (@b x) => 3
+let f13 = @a @b (~x) => 3


It looks printing okay now.

mununki · 2022-10-15T16:55:34Z

tests/printer/expr/expected/asyncAwait.res.txt

@@ -106,7 +106,7 @@ let _ = await {

 let f1 = async (~x, ~y) => x + y
 let f2 = async (@foo ~x, @bar ~y) => x + y
-let f3 = async (@bar @foo ~x as @zz z, ~y) => x + y
+let f3 = @foo async (~x as @bar @zz z, ~y) => x + y


I'm not sure about this expression @attr1 ~x as @attr2 y.
If we parse the attributes inside (...), the attributes are going to be ppat_attributes, the printer emits ~x as @attr1 @attr2 y What do you think?

Looks fine. There's only one place for attributes on labels.

mununki · 2022-10-15T17:05:49Z

tests/ppx/react/expected/topLevel.res.txt

-      Js.log("This function should be named 'TopLevel.react'")
-      ReactDOMRe.createDOMElementVariadic("div", [])
-    }
+    @warning("-16")


This warning is added to pexp_attributes by JSX v3.

Parsing and printing should not change anything for JSX right?

Actually, this is printing change. The ast is not changed.

I don’t think the printing changes of JSX output would be a problem unless the ast is changed. What do you think?
I’ll check the js ouput for this case.

JSX is never printed

mununki · 2022-10-16T04:29:37Z

I ran a test of the compiler with the syntax submodule checked out to this branch. The test result looks fine.

$ make test-all

IwanKaramazow · 2022-10-16T06:15:00Z

src/res_core.ml

@@ -1534,12 +1534,6 @@ and parseParameter p =
  then
    let startPos = p.Parser.startPos in
    let uncurried = Parser.optional p Token.Dot in
-    (* two scenarios:


Can we move this comment/mental model to the appropriate place?

f2a9720 How about this?

EDIT: f902e3a

mununki · 2022-10-16T13:32:47Z

Rebaed to master to resolve the confict in the test txt

cristianoc

Starting to look at this. For post v10.1 release.
Would you rebase on master?

cristianoc · 2022-11-04T09:59:34Z

src/res_core.ml

              Asttypes.Labelled lblName,
-              Ast_helper.Pat.var ~attrs:[propLocAttr] ~loc
+              Ast_helper.Pat.var ~attrs:([propLocAttr] @ attrs) ~loc


Use :: not @

cristianoc · 2022-11-04T10:01:00Z

src/res_core.ml

@@ -1569,25 +1569,29 @@ and parseParameter p =
            let pat =
              let pat = Ast_helper.Pat.var ~loc (Location.mkloc lblName loc) in
              let loc = mkLoc startPos p.prevEndPos in
-              Ast_helper.Pat.constraint_ ~attrs:[propLocAttr] ~loc pat typ
+              Ast_helper.Pat.constraint_ ~attrs:([propLocAttr] @ attrs) ~loc pat


Use :: not @

cristianoc · 2022-11-04T10:07:09Z

tests/printer/expr/expected/asyncAwait.res.txt

@@ -106,7 +106,7 @@ let _ = await {

 let f1 = async (~x, ~y) => x + y
 let f2 = async (@foo ~x, @bar ~y) => x + y
-let f3 = async (@bar @foo ~x as @zz z, ~y) => x + y
+let f3 = @foo async (~x as @bar @zz z, ~y) => x + y


Looks fine. There's only one place for attributes on labels.

- print attributes respectively

- of how to parse function parameters and attributes

mununki · 2022-11-04T15:13:28Z

Rebased to master

cristianoc · 2022-11-08T04:07:29Z

moved to compiler repo

mununki marked this pull request as draft October 15, 2022 10:22

mununki force-pushed the parse-attributes branch from 4b2a3b4 to d7f80a4 Compare October 15, 2022 16:47

mununki commented Oct 15, 2022

View reviewed changes

mununki requested review from cristianoc and IwanKaramazow October 15, 2022 16:55

mununki marked this pull request as ready for review October 15, 2022 16:58

mununki commented Oct 15, 2022

View reviewed changes

IwanKaramazow reviewed Oct 16, 2022

View reviewed changes

mununki force-pushed the parse-attributes branch from 0990331 to a567272 Compare October 16, 2022 13:32

mununki mentioned this pull request Oct 26, 2022

Tricky case for uncurried by default when . appears in non head positions rescript-lang/rescript#5792

Closed

cristianoc added this to the v11.0 milestone Oct 26, 2022

cristianoc mentioned this pull request Nov 2, 2022

Process uncurried types explicity. #717

Closed

cristianoc reviewed Nov 4, 2022

View reviewed changes

mununki added 4 commits November 5, 2022 00:11

parse attributes for function and arg respectively

3a158d5

- print attributes respectively

add comments for the mental model

79154ee

- of how to parse function parameters and attributes

update changelog

ce2a8de

use :: instead

779a1b8

mununki force-pushed the parse-attributes branch from a567272 to 779a1b8 Compare November 4, 2022 15:12

mununki mentioned this pull request Nov 7, 2022

Simplify parsing and printing attributes of function and arguments rescript-lang/rescript#5783

Merged

cristianoc closed this Nov 8, 2022

This was referenced Dec 14, 2022

Fix dropping attributes of props in JSX v4 rescript-lang/rescript#5905

Merged

Simplify parsing attributes of function and args #722

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simplify parsing and printing attributes of function and arguments #683

Simplify parsing and printing attributes of function and arguments #683

mununki commented Oct 15, 2022

mununki commented Oct 15, 2022

cristianoc commented Oct 15, 2022

cristianoc commented Oct 15, 2022

mununki commented Oct 15, 2022

mununki Oct 15, 2022

mununki Oct 15, 2022

cristianoc Nov 4, 2022

mununki Oct 15, 2022

cristianoc Oct 15, 2022

mununki Oct 15, 2022

mununki Oct 15, 2022 •

edited

Loading

cristianoc Oct 15, 2022

mununki commented Oct 16, 2022

IwanKaramazow Oct 16, 2022

mununki Oct 16, 2022 •

edited

Loading

mununki commented Oct 16, 2022

cristianoc left a comment

cristianoc Nov 4, 2022

cristianoc Nov 4, 2022

cristianoc Nov 4, 2022

mununki commented Nov 4, 2022

cristianoc commented Nov 8, 2022

Simplify parsing and printing attributes of function and arguments #683

Simplify parsing and printing attributes of function and arguments #683

Conversation

mununki commented Oct 15, 2022

mununki commented Oct 15, 2022

cristianoc commented Oct 15, 2022

cristianoc commented Oct 15, 2022

mununki commented Oct 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mununki Oct 15, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mununki commented Oct 16, 2022

Choose a reason for hiding this comment

mununki Oct 16, 2022 • edited Loading

Choose a reason for hiding this comment

mununki commented Oct 16, 2022

cristianoc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mununki commented Nov 4, 2022

cristianoc commented Nov 8, 2022

mununki Oct 15, 2022 •

edited

Loading

mununki Oct 16, 2022 •

edited

Loading